@bitmold, it's about time you have a look at this.

Since you know the Tor-controlling part way better than me: What would you consider the best way to implement the test?

Additionally, I would be happy for any testing against non-standard phones and tablets.

Thank you!

please rebase

Awesome, I'll take a look at it tonight + tomorrow morning.

Something is strange with the latest merge. This commit tladesignz@b070bf1 changes String to String?, but then the following change to override fun natTypeUpdated in SnowflakeProxyWrapper.kt is 6bfcb7e which shows String not String?, but this does not match HEAD.

Also, kindness_mode_disclaimer_bridge should be removed from app/src/main/res/values-tr/strings.xml

diff --git a/app/src/main/java/org/torproject/android/ui/kindness/SnowflakeProxyWrapper.kt b/app/src/main/java/org/torproject/android/ui/kindness/SnowflakeProxyWrapper.kt
index eefd62d14..6ae9cc305 100644
--- a/app/src/main/java/org/torproject/android/ui/kindness/SnowflakeProxyWrapper.kt
+++ b/app/src/main/java/org/torproject/android/ui/kindness/SnowflakeProxyWrapper.kt
@@ -89,8 +89,8 @@ class SnowflakeProxyWrapper(private val service: SnowflakeProxyService) {
                         // Ignored.
                     }
 
-                    override fun natTypeUpdated(natType: String?) {
-                        // TODO feature added in IPtProxy 5.4.1
+                    override fun natTypeUpdated(natType: String) {
+                        service.updateNatType(natType)
                     }
                 }
 
diff --git a/app/src/main/res/values-tr/strings.xml b/app/src/main/res/values-tr/strings.xml
index 695dc397c..bd94cd87a 100644
--- a/app/src/main/res/values-tr/strings.xml
+++ b/app/src/main/res/values-tr/strings.xml
@@ -277,7 +277,6 @@
     <string name="kindness_never_had_a_direct_connection">Paylaşım Modunu kullanabilmek için, en az bir kez Tor ağına doğrudan (herhangi bir köprü kullanmadan) bağlanmanız gerekir. Bağlantı kurduktan sonra, lütfen buraya geri dönerek Paylaşım Modunu etkinleştirin. Bu doğrudan bağlantı testini tamamladıktan sonra, Tor ağına bağlı olsanız da olmasanız da Paylaşım Modunu dilediğiniz gibi kullanabilirsiniz.</string>
     <string name="kindness_mode_cant_run_with_bridge">Tor ağına bağlanmak için bir köprü kullanırken Paylaşım Modu çalıştırılamaz.</string>
     <string name="kindness_mode_cant_run_in_your_country">Sansür nedeniyle, ülkenizde diğer kişilerin Tor ağına bağlanmasına yardımcı olmak için kullanılan Paylaşım Modu şu anda devre dışıdır.</string>
-    <string name="kindness_mode_disclaimer_bridge">Sansür uygulanan bir bölgedeyseniz veya Tor ağına bağlanmak için bir köprü kullanmanız gerekiyorsa, Paylaşım Modunu kullanmayın.</string>
     <string name="battery_optimization_title">Pil İyileştirmelerini Devre Dışı Bırak</string>
     <string name="battery_optimization_pref_msg">Aygıtlarının Orbot uygulamasını arka planda çalışırken erken bir şekilde kapattığından şüphelenen kullanıcılar için ayar.</string>
     <string name="battery_optimization_dialog_msg">Bazı Android aygıtlar, Orbot arka planda çalışırken uygulamayı kapatabilir. Orbot\'un bağlantısının kesilmemesini ve çalışmaya devam etmesini isteyen çoğu kullanıcı için, sistem ayarlarından Orbot\'u \"Her Zaman Açık\" VPN olarak ayarlamak yeterlidir. Bununla birlikte, gerekli görürseniz, aygıtınızın Orbot üzerinde herhangi bir pil iyileştirmesi yapmasını engellemek için gerekli adımları atabilirsiniz.</string>

Noting this regression here, though I understand that this dialog that checks if a direct tor connection has been established is being replaced in some way by this testing dialog.

When you click activate on a cold install, the dialog to ensure that you have at least historically have had a direct tor connection appears. At this moment snowflake proxy is running and there's no way to turn it off.

If I go ahead and connect to tor, then go to activate kindness mode, the bug still exists. The testing dialog appears, but kindness mode is already running.

On small screens, the headings get cramped

The connected UI looks great, but unfortunately it breaks in landscape

And this is a bit nitpicky, but the labels aren't lining up with these grey panels

Also @cstiens it just dawned on me that on the kindness screen rectangular grey panels are used, whereas on the "more" screen to the right rounded panels are used. Personally, I think the slightly rounded ones look much nicer.

Noting this regression here, though I understand that this dialog that checks if a direct tor connection has been established is being replaced in some way by this testing dialog.

When you click activate on a cold install, the dialog to ensure that you have at least historically have had a direct tor connection appears. At this moment snowflake proxy is running and there's no way to turn it off.

If I go ahead and connect to tor, then go to activate kindness mode, the bug still exists. The testing dialog appears, but kindness mode is already running.

Good catch! I removed the legacy test.

• Fixed the header indent.
• Added rounded cards as suggested.
• Added first test code and a mock test to be able to continue development.
• Added final behaviour to show initial scene only, when no valid test result. (older than 24 hours.)

Found another problem:

We need to bind to the SnowflakeProxyService to receive the evaluation of the proxy quality.

However, doing so unintentionally starts a SnowflakeProxy. I'm afraid, SnowflakeProxyService is in for a redesign. @bitmold, what do you think?

We need to bind to the SnowflakeProxyService to receive the evaluation of the proxy quality.
However, doing so unintentionally starts a SnowflakeProxy. I'm afraid, SnowflakeProxyService is in for a redesign. @bitmold, what do you think?

This is true, it needs to be modified, however I don't think a thorough redesign is needed.

SnowflakeProxyService is capable of running as a foreground service without actually running the snowflake proxy in IPtProxy. For instance, if the Service is configured to run on WiFi only and/or if the device is connected to a power source, the Service is capable of calling stopSnowflakeProxy(String?) to turn off the proxying.

When the Service is created in onCreate() it calls initNetworkCallbacks() which immediately starts proxying if the network conditions are met. In this function you could add preconditions to ensure that the correct proxy evaluations are met before invoking startSnowflakeProxy(String?)

If, in onStart() we jiust want to show the Fragment's NAT type value when kindness mode is turned On, that's easy. OrbotActivity can have a BroadcastReceiver that we use when binding to SnowflakeProxyService. Whenever we get a new NAT type from IPtProxy, it sends an update to the Activity which stores it in its view model. The Fragment, can query this value whenever it's opened up without needing to directly communicate to the Service.

Another, i had another idea that came to mind just now... Every time we get a new network event, (gain Wi-Fi, revert to cellular, connect to a new Wi-Fi, etc) SnowflakeProxyWrapper can be modified to stop and then restart snowflake proxy. This causes the initial NAT Type evaluation logic that happens when snowflake proxy first starts to fire off on every Android network event. You'd get accurate + timely NAT type updates for whatever connection you're currently on as soon as you connect to it (provided that kindness mode is turned on, of course). To me it seems better than finding an optimal repeat interval for NATTypeMeasurementInterval. As stated above, we already do something like this if the user has a Wi-Fi/power restriction. The difference is that rather than disconnecting when you're on Wi-Fi only and drop to a cellular network, you instead always disconnect on a new network, and then either immediately restart snowflake proxy if conditions are met/absent, or otherwise do what it already does today and wait to start snowflake proxy back up @ some future point after the device rejoins a Wi-Fi network.

Alternatively, is there anyway snowflake.go could be patched/modified for to accept a signal that on-demand fires off its NAT evaluation instead of us starting/stopping it in Orbot. This seems like the most elegant approach instead of my above idea ofkilling/relaunching SnowflakeProxyService on new network events, or using a polling interval.

@bitmold re NAT quality evaluation:

Wow, you quite overthought this a lot.

No, I don't want to start Snowflake Proxy just to get the NAT quality info.

I just want to get that info at all, when it is available. My first approach was to bind to the service and get the value communicated that way, but that turned out to be a bad idea, because that would actually start the Snowflake Proxy as the service is currently set up.

Your idea of rather using a local broadcast of course also works. AFAIR, that is discouraged though since recently and my Android Studio complains about it.

But, if you think to stay on local broadcasts is better than to redesign the service so it doesn't automatically start the Snowflake Proxy, I'm fine.

I like the idea of restarting the Snowflake Proxy on network changes.

And thanks for the hin with the NATTypeMeasurementInterval. I totally forgot about it and was actually waiting that something like that gets implemented. 🤣 I'll use that on iOS, as there it's more complicated to listen for network changes.

I still think that showing NATUnknown when the Snowflake Proxy connection stops is fine. I wouldn't want to show/hide whole elements all the time. That seems to bring a sort of restlessness into the UI.

No, I don't want to start Snowflake Proxy just to get the NAT quality info.

I just want to get that info at all, when it is available. My first approach was to bind to the service and get the value communicated that way, but that turned out to be a bad idea, because that would actually start the Snowflake Proxy as the service is currently set up.

@tladesignz my confusion is there's currently a view displaying the NAT type, introduced by this PR, when kindness mode is turned off. Why even show it? Since the only way for the NAT type to be available is after starting snowflake proxy, correct?

Or to put it another way, the only way to get the NAT type in snowflake.go is to start proxying, it tells you the NAT type when the proxy first starts, and from there, it optionally can do the test at fixed intervals.

Curious on your thoughts for a reasonable NATTypeMeasurementInterval value. Is 5 minutes too much, too little?

@tladesignz my confusion is there's currently a view displaying the NAT type, introduced by this PR, when kindness mode is turned off. Why even show it? Since the only way for the NAT type to be available is after starting snowflake proxy, correct?

Or to put it another way, the only way to get the NAT type in snowflake.go is to start proxying, it tells you the NAT type when the proxy first starts, and from there, it optionally can do the test at fixed intervals.

As I already stated above:

I still think that showing NATUnknown when the Snowflake Proxy connection stops is fine. I wouldn't want to show/hide whole elements all the time. That seems to bring a sort of restlessness into the UI.

Curious on your thoughts for a reasonable NATTypeMeasurementInterval value. Is 5 minutes too much, too little?

For iOS/macOS I decided upon 1 hour. Since it's a foreground thing on iOS, I suppose user's are not going to move much while running the Snowflake Proxy.

Of course, that can be very different on Android. But 5 minutes still seems a little much.

Since there's no way to simply ask snowflake to do the test for us on demand, I'm going to take 20 min and implement restarting it on network change, and then test it by biking to around to several nearby friends' front porches (I'm on their Wi-Fi networks). if this works, we'll go with it. If not, I might try something like NATTypeMeasurementInterval = 30 minutes 🤷‍♀️

For mac an hour seems perfect though

That's why I always pushed for the ultimate test: Try to run a straight Tor connection. That's the actual always true fact upon which we can decide if running a Snowflake Proxy makes sense or not.
How about this:
There is certainly the case, that for some people, it could theoretically be detrimental to get their Tor connection closed and a straight Tor connection established.
In the current implementation, this is somewhat surprising.
We certainly need some interstitial alert before actually starting the test.
That seems to me the right place to use the country information.
"We need to run a straight Tor connection as a test to see, if your current location is a good place for running a Snowflake Proxy. We will now disconnect your Tor VPN and run that test.

I didn't mean to suggest that those countries were an exhaustive list of regions where you can't run snowflake. I made the change after it seemed like a user of the iOS app based in Turkey complained after failing the test. Of course, in all the countries not in that list, if you try to start kindness mode you have the opportunity to pass or fail the definitive test.

The idea was to save users who want to just get online to talk to their families or read the news time by blocking off part of this app because even though there's been all this amazing UX work put into Orbot, the app still is still inevitably filled with a lot of confusing terminology and technical concepts that most users do not have a solid understanding of.

I'd rather just block it in some places where it's almost always not going to work anytime soon, rather than say something to the effect of

"You selected XXX as your country. Running a Snowflake Proxy in that country most probably does not make sense. You cannot help others here. STOP RIGHT NOW."

I think many users would find this warning confusing and scary. To me, keeping things streamlined and uncomplicated for users who have chosen to specify that they're in a sensitive area vastly outweighs being able to on ramp the rare starlink user in say Afghanistan, for example. And even if If they're clandestinely using starlink against the violent morality laws of the Taliban, then it's safe to assume that they probably have bigger concerns than helping users connect to the tor network via Orbot's kindness mode. If I had to guess, the lion's share of people in that region who are able to pass the test and sustainably run kindness mode would probably be some less than tech-savvy people in proximity to the Supreme Leader that have Orbot installed and then randomly turned on kindness mode and are somewhat unknowingly dedicating bandwidth to help people over the world connect to Tor.

But say you do have Starlink in a violently censored place and do use kindness mode - well for those, what 2 dozen users, having kindness mode on is only useful while they're on that starlink network, the minute they go 6-15ft (1.8-4.5m) away from the router and possibly join another network then they're running kindness mode uselessly, while perhaps also jeopardizing their personal safety.

As for someone on a privileged university Wi-Fi the same drawback is the case. Maybe, this very negligible amount of users could be snowflakes, but they're still going to become immediately unhelpful proxies the minute they leave their campus network. I would imagine this hypothetical researcher would probably be more drawn to the standalone proxy anyway.

As for white SIM card holders in Iran, my understanding was those SIMs were issued to those aligned with IRGC. Since it seems like you have to be aligned with the state's project of censorship to get one of those SIM cards, I don't think those Orbot users would be interested in helping to run a technology that's historically been used for circumvention in Iran.

How about this:

• we remove some countries from the list I borrowed from DNSTT to only include ones where it's been and probably will continue to be a statistical improbability that someone could effectively use kindness mode in a sustained and helpful way. Off the top of my head, Afghanistan and Iran come to mind as ones to keep, but I'm sure there are a few more.
• if a user selected that they are in a country on this list, instead of saying my proposed "Thank you so much for your interest in helping others connect to Tor. Unfortunately, Kindness Mode isn't available in your country at this time. We appreciate your generosity." we take your suggestion "You selected XXX as your country. Running a Snowflake Proxy in that country most probably does not make sense. You cannot help others here. STOP RIGHT NOW." and modify it to ➡️ "You selected XXX as your country. Running a Snowflake Proxy in that country most probably does not make sense. If you still want to try to see if your device is capable of being a Snowflake Proxy, please clear your selected country." It's less scary that way, and it gives users who really want to do this the option to try the test just like everyone else, even though they'd likely fail it.

I 100% agree with you though that the testing dialog should have some kind of interstitial warning like you mentioned in that comment I just quoted

Since there's no way to simply ask snowflake to do the test for us on demand, I'm going to take 20 min and implement restarting it on network change, and then test it by biking to around to several nearby friends' front porches (I'm on their Wi-Fi networks). if this works, we'll go with it. If not, I might try something like NATTypeMeasurementInterval = 30 minutes 🤷‍♀️

For mac an hour seems perfect though

Love your physical dedication! 😁

@tladesignz this is a WIP. I will ping you when it's ready for review. There are a lot of tricky corner cases on this. It'll be done before Tuesdays meeting